fix: resolve sort comparator and code quality issues (@ennajari)#7805
fix: resolve sort comparator and code quality issues (@ennajari)#7805ennajari wants to merge 7 commits intomonkeytypegame:masterfrom
Conversation
- Use localeCompare in sort calls to ensure consistent ordering across locales (setters.ts, Sidebar.tsx, connections.ts, themes.ts) - Add initial value 0 to reduce() in stdDev to handle empty arrays safely - Use explicit undefined check for cached Promise in memoize utility - Add NOSONAR annotations for intentional patterns (thenable object, CharacterCounter side-effect instantiation) - Add .scannerwork/, .claude/, sonar-project.properties to .gitignore
monkeytypegeorge
added
backend
ennajari
changed the title
backend/src/dal/connections.ts
Outdated
| function getKey(initiatorUid: string, receiverUid: string): string { | ||
| const ids = [initiatorUid, receiverUid]; | ||
| ids.sort(); | ||
| ids.sort((a, b) => a.localeCompare(b)); |
|
We don't use sonar, please remove sonar instructions and .ignore |
frontend/src/ts/config/setters.ts
Outdated
| } else { | ||
| newConfig.push(funbox); | ||
| newConfig.sort(); | ||
| newConfig.sort((a, b) => a.localeCompare(b)); |
There was a problem hiding this comment.
I think we want consistent sorting here and not depend on the users locale.
There was a problem hiding this comment.
Changed to a locale-independent comparator: (a, b) => (a < b ? -1 : a > b ? 1 : 0). Same for Sidebar.tsx and themes.ts.
There was a problem hiding this comment.
Could you describe the difference between
Object.keys(themes).sort()and
Object.keys(themes).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))There was a problem hiding this comment.
Honestly, there is no functional difference. For ASCII strings like theme/funbox names, both perform Unicode code-point comparison and produce identical results. The explicit comparator only satisfies the static analysis rule that requires a compare function to be provided, but the behavior is the same as plain .sort().
If you'd prefer, I can drop these three sort changes entirely — the two remaining fixes (reduce initial value and the !== undefined check) still stand on their own.
There was a problem hiding this comment.
If both do the same let's keep the simple .sort
There was a problem hiding this comment.
Reverted all three sort changes back to plain .sort(). The PR now only contains the reduce initial value fix and the !== undefined cache check.
|
Hi @ennajari , thanks for the changes. The pr summary doesn't match the or content anymore |
github-actions
bot
added
waiting for review
3 participants
Summary
0toreduce()instdDevto safely handle edge cases!== undefinedcheck instead of truthy check for cachedPromisein memoize utilityTest plan
stdDev([])returns0instead ofNaN